Skip to content

Warn on older versions of func#3

Merged
gauron99 merged 1 commit intomainfrom
push-loruqtwsqvmw
Feb 3, 2026
Merged

Warn on older versions of func#3
gauron99 merged 1 commit intomainfrom
push-loruqtwsqvmw

Conversation

@gauron99
Copy link
Collaborator

@gauron99 gauron99 commented Jan 29, 2026 •
edited
Loading

This is a subsequent PR to #2

  • warn on func version when version is <= 3 than latest (latest - 3 and older)
  • one "practical" test in workflow for simplicity (can check manually in the workflow which uses custom version)

please note

  • when someone specifies the custom binary url base we skip the version check (dont warn)
  • when someone specifies 'latest' version we skip the version check (dont warn)

will add in subsequent PR

  • some js tests to cover this ( I want to remove the node_modules first -- use ncc module )

@gauron99 gauron99 force-pushed the push-loruqtwsqvmw branch 3 times, most recently from bafd5a7 to 54b8989 Compare January 30, 2026 10:48
twoGiants
twoGiants suggested changes Jan 30, 2026
View reviewed changes
Copy link

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! 👍

I think we can make it a bit simpler. Wdyt?

See my comments below.

twoGiants
twoGiants suggested changes Feb 3, 2026
View reviewed changes
Copy link

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, looks good! Your JavaScript muscles are getting bigger 💪 😄 , love it!

I left a few comments and would do another round. Hope it's ok! We're close to the finish line 🎸 🎶 🎶

Let me know if anything is unclear.

By the way => next PR should be unit test coverage. Logic is getting more and more complex.

index.js
Copy link

@twoGiants twoGiants Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, looks good, but lets clean it up a bit and then we're done. I hope it's ok for you to go another round 🙈 .

I would avoid working with null values if I can. Especially in JavaScript. Being explicit makes the code easier to reason about, at least for me 😸. JavaScript is a permissive language 😆 it has undefined, null, NaN as empty/null values and they behave differently 😆. Unlike in go where nil checks are common place => in JavaScript I would use them only when there is no other way. So in the JS code you write, where you can set non-null values => do it and check against that.

twoGiants
twoGiants approved these changes Feb 3, 2026
View reviewed changes
Copy link

@twoGiants twoGiants left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 😸 👍

Thanks for keeping it clean and readable. 🥇

@gauron99 gauron99 merged commit 90f77ec into main Feb 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@twoGiants twoGiants twoGiants approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gauron99 @twoGiants